Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make the number of workers processing federated query configurable #6449

Conversation

SungJin1212
Copy link
Contributor

@SungJin1212 SungJin1212 commented Dec 20, 2024

This PR adds a -tenant-federation.max-concurrent flags to make the number of workers processing federated query configurable. and add a cortex_querier_federated_tenants_per_query histogram to track the number of tenants per query.

Which issue(s) this PR fixes:
Fixes #

Checklist

  • Tests updated
  • Documentation added
  • CHANGELOG.md updated - the order of entries should be [CHANGE], [FEATURE], [ENHANCEMENT], [BUGFIX]

@dosubot dosubot bot added component/querier type/observability To help know what is going on inside Cortex labels Dec 20, 2024
@SungJin1212 SungJin1212 force-pushed the Add-number-of-tenants-per-query-histogram branch from d5beaf8 to c124391 Compare December 20, 2024 10:07
@yeya24
Copy link
Contributor

yeya24 commented Dec 20, 2024

Maybe I don't have the scenario of query federation. What's the usecase for this metric?

@SungJin1212 SungJin1212 force-pushed the Add-number-of-tenants-per-query-histogram branch from d2c95c6 to 5e697d5 Compare December 20, 2024 21:26
@SungJin1212 SungJin1212 changed the title Add number tenants per query histogram Make the number of workers processing federated query configurable Dec 20, 2024
@SungJin1212
Copy link
Contributor Author

@yeya24
This PR is for making the number of workers processing federated query configurable.
It could help users using more than 16 tenants.
I have updated the PR.

@SungJin1212 SungJin1212 force-pushed the Add-number-of-tenants-per-query-histogram branch from 5e697d5 to fc594df Compare December 20, 2024 21:30
}

func (cfg *Config) RegisterFlags(f *flag.FlagSet) {
f.BoolVar(&cfg.Enabled, "tenant-federation.enabled", false, "If enabled on all Cortex services, queries can be federated across multiple tenants. The tenant IDs involved need to be specified separated by a `|` character in the `X-Scope-OrgID` header (experimental).")
f.IntVar(&cfg.MaxConcurrent, "tenant-federation.max-concurrent", defaultMaxConcurrency, "The number of workers used for processing federated query.")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the number of workers for a single federated query, correct? The description is a bit misleading as it sounds like concurrency for shared for all federated queries

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what about The number of workers used to process each federated query.?

@SungJin1212 SungJin1212 force-pushed the Add-number-of-tenants-per-query-histogram branch from fc594df to 7b8783b Compare December 22, 2024 01:58
Copy link
Member

@friedrichg friedrichg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks!

@dosubot dosubot bot added the lgtm This PR has been approved by a maintainer label Dec 24, 2024
@friedrichg friedrichg merged commit 452e144 into cortexproject:master Dec 24, 2024
17 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component/querier lgtm This PR has been approved by a maintainer size/L type/observability To help know what is going on inside Cortex
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants